[feat] Enable the hash join to accept a pre-built hash table for joining#150
Open
JkSelf wants to merge 1 commit intobytedance:mainfrom
Open
[feat] Enable the hash join to accept a pre-built hash table for joining#150JkSelf wants to merge 1 commit intobytedance:mainfrom
JkSelf wants to merge 1 commit intobytedance:mainfrom
Conversation
|
|
Collaborator
|
@JkSelf Thx 4 the contribution! Can you add unit tests to cover the change |
Collaborator
|
@JkSelf Can you explain this benchmark in more detail? For example, what is the test dataset and what is the scale factor? Is it a gain for a single query or an overall gain? Also, please sign the Contributor License, thank you! |
yangzhg
reviewed
Jan 19, 2026
| * -------------------------------------------------------------------------- | ||
| */ | ||
|
|
||
| #include <boost/sort/pdqsort/pdqsort.hpp> |
Collaborator
There was a problem hiding this comment.
why move boost headers to first?
Contributor
There was a problem hiding this comment.
Why move boost headers to first?
It may be clang-format that sorted the header files.
yangzhg
reviewed
Jan 19, 2026
|
|
||
| const bool nullAware_; | ||
|
|
||
| void* reusedHashTableAddress_; |
Collaborator
There was a problem hiding this comment.
This will made HashJoinNode cannot be serialized across process and machines. So better to add some comment or CHECK
yangzhg
reviewed
Jan 19, 2026
| std::unique_ptr< | ||
| exec::BaseHashTable, | ||
| std::function<void(exec::BaseHashTable*)>> | ||
| hashTable(nullptr, [](exec::BaseHashTable* ptr) { /* Do nothing */ }); |
yangzhg
reviewed
Jan 19, 2026
|
|
||
| // True if this is a build side of an anti or left semi project join and has | ||
| // at least one entry with null join keys. | ||
| bool joinHasNullKeys_{false}; |
Collaborator
There was a problem hiding this comment.
Suggested change
| bool joinHasNullKeys_{false}; | |
| std::atomic<bool> joinHasNullKeys_{false}; |
JkSelf
force-pushed
the
bhj-optimization
branch
from
4b21891 to
730f908
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #149
Type of Change
Description
In Spark, the hash table is constructed only once in the driver and then broadcasted to each executor. In Gluten, we replace the broadcast hash join with a hash join, which leads to performance issues when the broadcast threshold increases. This is because each task must build its own hash table when using hash join.
This PR enables HashBuild to accept pre-built HashTable, thereby bypassing the hash table construction process. Additionally, it modifies HashProbe to avoid clearing the shared hash table after use.
Performance Impact
No Impact: This change does not affect the critical path (e.g., build system, doc, error handling).
Positive Impact: I have run benchmarks.
Click to view Benchmark Results
Negative Impact: Explained below (e.g., trade-off for correctness).
Release Note
Please describe the changes in this PR
Release Note:
Checklist (For Author)
Breaking Changes
No
Yes (Description: ...)
Click to view Breaking Changes